-
Notifications
You must be signed in to change notification settings - Fork 12
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
extensions: overhaul javadoc/doxygen comment parsing #216
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow... You know, this may well be the most impactful commit yet, which is kind of ironic because it's an extension neither of us personally care about that much 😅
Didn't go about checking this against Doxygen documentation or anything or even testing things beyond the tests we have so far, but looks alright. No notes.
Not sure how confident you are on merging this right ahead... Are there any known users? Can we get them to try it out?
Indeed!
I really didn't expect that either. The number of doxygen commands blew my mind. And kind of reaffirmed my belief that if you want to incorporate API documentation to Sphinx, Hawkmoth is the way to go!
I do want to add more tests before merging. Probably not for everything, because that's just too overwhelming, but maybe I'll sample the source for some of the highlighted projects at https://doxygen.nl/results.html and make sure the popular things work. I already looked at dbus, and I was actually surprised how basic the usage was. Another approach is to look at coverage and add targeted tests to increase coverage; there's quite a bit of reuse for the "handlers". On the other hand, the old one was so rudimentary that this can't possibly be worse! 😆 Anyway, I did ask for feedback at #210 (comment) |
👍
That actually doesn't surprise me. I expect most projects are very frugal with what they use out of discipline / consistency but also probably due to no one being able to remember and use all those things effectively. Maybe you can look at doxygen itself? That might be the most comprehensive example. In fact they may have unit tests of their own we can poach, license allowing.
Yeah, though it does beg the age-old question as to what coverage tells us apart from identifying dead code <.<
Bold last words, but yeah :P |
Yeah, there's a license conflict. Can't use GPL2 code in a BSD licensed project. |
Full javadoc/doxygen compatibility has never been a goal for hawkmoth. We've always promoted using pure reStructuredText and Sphinx, because it avoids any problematic conversions. If you want all the bells and whistles of doxygen, you should use doxygen. However, we have to acknowledge there are a lot of codebases full of javadoc/doxygen style documentation comments, and a lot of people who are familiar with that style of code documentation. Requiring the use of reStructuredText to even try hawkmoth can be quite a hurdle. To that end, we've always had a rudimentary regex based conversion available. But let's face the fact, is too simple, and too difficult to maintain or extend as it is. Try to find a middle ground with an improved parser that understands paragraphs, inline markup, code blocks, and the like. Make it easier to extend. Recognize all doxygen commands (more than 180 of them!), even though we only implement a handful, making it possible to warn about the unimplemented ones (this is for future improvement, not done yet).
Improve the description of the javadoc builtin extension. This also expands the tests slightly.
It's a little better than "rudimentary at best", but still keep the expectations at bay.
Slightly tweaked the inline markup regexps, dropped the "split field list" thing after experimenting with what doxygen actually does, and added some documentation commits on top. I expanded the example, which also means it slightly expands the test coverage. I could add a lot more tests, but not sure I want to put a huge amount of effort into it yet. |
Looks good to me. |
Thanks! Let's merge, and iterate from there as needed! |
Tested it on my current project, and it works perfectly 🎉 |
That's awesome! Thanks for testing. |
Okay, I never thought I'd put much effort into this, but the current javadoc/doxygen one is just embarrasing. Some might argue not implementing a parser using a parser generator is also embarrassing, but I'm really not aiming that high. A few hundred lines of bespoke parser to bridge the gap should be good enough.
Full javadoc/doxygen compatibility has never been a goal for hawkmoth. We've always promoted using pure reStructuredText and Sphinx, because it avoids any problematic conversions. If you want all the bells and whistles of doxygen, you should use doxygen.
However, we have to acknowledge there are a lot of codebases full of javadoc/doxygen style documentation comments, and a lot of people who are familiar with that style of code documentation. Requiring the use of reStructuredText to even try hawkmoth can be quite a hurdle.
To that end, we've always had a rudimentary regex based conversion available. But let's face the fact, is too simple, and too difficult to maintain or extend as it is.
Try to find a middle ground with an improved parser that understands paragraphs, inline markup, code blocks, and the like. Make it easier to extend. Recognize all doxygen commands (more than 180 of them!), even though we only implement a handful, making it possible to warn about the unimplemented ones (this is for future improvement, not done yet).